sim: call `.aclose()` on `TickTrigger` in `.until()` and `.repeat()` #1590

whitequark · 2025-05-05T04:18:53Z

Otherwise, when used together with asyncio, the following warning will be printed for each use of .until()/.repeat():

E: asyncio: Task was destroyed but it is pending!
task: <Task pending name='Task-2' coro=<<async_generator_athrow without __name__>()>>
/usr/lib/python3.13/asyncio/base_events.py:744: RuntimeWarning: coroutine method 'aclose' of 'TickTrigger.__aiter__' was never awaited

Otherwise, when used together with asyncio, the following warning will be printed for each use of `.until()`/`.repeat()`: E: asyncio: Task was destroyed but it is pending! task: <Task pending name='Task-2' coro=<<async_generator_athrow without __name__>()>> /usr/lib/python3.13/asyncio/base_events.py:744: RuntimeWarning: coroutine method 'aclose' of 'TickTrigger.__aiter__' was never awaited

mabl · 2025-06-27T08:59:52Z

I am unfortunately running into random failures with this change in my testcases:

  + Exception Group Traceback (most recent call last):
  |   File "C:\path\.venv\Lib\site-packages\_pytest\runner.py", line 344, in from_call
  |     result: TResult | None = func()
  |                              ^^^^^^
  |   File "C:\path\.venv\Lib\site-packages\_pytest\runner.py", line 246, in <lambda>
  |     lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "C:\path\.venv\Lib\site-packages\pluggy\_hooks.py", line 513, in __call__
  |     return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "C:\path\.venv\Lib\site-packages\pluggy\_manager.py", line 120, in _hookexec
  |     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "C:\path\.venv\Lib\site-packages\pluggy\_callers.py", line 139, in _multicall
  |     raise exception.with_traceback(exception.__traceback__)
  |   File "C:\path\.venv\Lib\site-packages\pluggy\_callers.py", line 122, in _multicall
  |     teardown.throw(exception)  # type: ignore[union-attr]
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "C:\path\.venv\Lib\site-packages\_pytest\logging.py", line 850, in pytest_runtest_call
  |     yield
  |   File "C:\path\.venv\Lib\site-packages\pluggy\_callers.py", line 122, in _multicall
  |     teardown.throw(exception)  # type: ignore[union-attr]
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "C:\path\.venv\Lib\site-packages\_pytest\capture.py", line 900, in pytest_runtest_call
  |     return (yield)
  |             ^^^^^
  |   File "C:\path\.venv\Lib\site-packages\pluggy\_callers.py", line 122, in _multicall
  |     teardown.throw(exception)  # type: ignore[union-attr]
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "C:\path\.venv\Lib\site-packages\_pytest\skipping.py", line 263, in pytest_runtest_call
  |     return (yield)
  |             ^^^^^
  |   File "C:\path\.venv\Lib\site-packages\pluggy\_callers.py", line 103, in _multicall
  |     res = hook_impl.function(*args)
  |           ^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "C:\path\.venv\Lib\site-packages\_pytest\unraisableexception.py", line 158, in pytest_runtest_call
  |     collect_unraisable(item.config)
  |   File "C:\path\.venv\Lib\site-packages\_pytest\unraisableexception.py", line 81, in collect_unraisable
  |     raise ExceptionGroup("multiple unraisable exception warnings", errors)
  | ExceptionGroup: multiple unraisable exception warnings (2 sub-exceptions)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "C:\path\.venv\Lib\site-packages\amaranth\sim\_async.py", line 358, in until
    |     clk, rst, *values, done = await tick.__anext__()
    |                               ^^^^^^^^^^^^^^^^^^^^^^
    | GeneratorExit
    |
    | During handling of the above exception, another exception occurred:
    |
    | Traceback (most recent call last):
    |   File "C:\path\tests\protocol\test_parse_recordings.py", line 78, in testbench_output_no_error
    |     await ctx.tick().until(dut.error_state == 1)
    |   File "C:\path\.venv\Lib\site-packages\amaranth\sim\_async.py", line 363, in until
    |     await tick.aclose()
    | RuntimeError: aclose(): asynchronous generator is already running
    |
    | The above exception was the direct cause of the following exception:
    |
    | Traceback (most recent call last):
    |   File "C:\path\.venv\Lib\site-packages\_pytest\unraisableexception.py", line 67, in collect_unraisable
    |     warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))
    | pytest.PytestUnraisableExceptionWarning: Exception ignored in: <coroutine object TestParseRecordings.test_parse_recordings.<locals>.testbench_output_no_error at 0x000002C4FE47B8A0>
    | Enable tracemalloc to get traceback where the object was allocated.
    | See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.
    +---------------- 2 ----------------
    | Traceback (most recent call last):
    |   File "C:\path\.venv\Lib\site-packages\amaranth\sim\_async.py", line 358, in until
    |     clk, rst, *values, done = await tick.__anext__()
    |                               ^^^^^^^^^^^^^^^^^^^^^^
    | GeneratorExit
    |
    | During handling of the above exception, another exception occurred:
    |
    | Traceback (most recent call last):
    |   File "C:\path\tests\protocol\test_parse_recordings.py", line 68, in testbench_output
    |     await stream_get(ctx, dut.o_stream)
    |   File "C:\path\tests\helpers\stream.py", line 8, in stream_get
    |     payload, = await ctx.tick().sample(s.payload).until(s.valid)
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |   File "C:\path\.venv\Lib\site-packages\amaranth\sim\_async.py", line 363, in until
    |     await tick.aclose()
    | RuntimeError: aclose(): asynchronous generator is already running
    |
    | The above exception was the direct cause of the following exception:
    |
    | Traceback (most recent call last):
    |   File "C:\path\.venv\Lib\site-packages\_pytest\unraisableexception.py", line 67, in collect_unraisable
    |     warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))
    | pytest.PytestUnraisableExceptionWarning: Exception ignored in: <coroutine object TestParseRecordings.test_parse_recordings.<locals>.testbench_output at 0x000002C4989AEB20>
    | Enable tracemalloc to get traceback where the object was allocated.
    | See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.
    +------------------------------------

I'm wondering, why the code needs to be that complex? Can't we simply do

        async for clk, rst, *values, done in self.sample(condition):
            if rst:
                raise DomainReset
            if done:
                return tuple(values)

That also fixes the problem for me.

The other code segment would probably be written as

        count = operator.index(count)
        if count <= 0:
            raise ValueError(f"Repeat count must be a positive integer, not {count!r}")

        result = None
        async for i, (clk, rst, *values) in enumerate(self):
            if i >= count:
                break
            if rst:
                raise DomainReset
            assert clk
            result = tuple(values)
        return result

but I have not tested that.

whitequark · 2025-06-27T09:01:46Z

Yes, I've seen these failures too.

whitequark · 2025-06-27T09:19:33Z

Would you like to submit your changes as a PR?

mabl · 2025-06-27T09:29:24Z

Yes, I've seen these failures too.

Alright, glad I don't need to build a simplified test case to generate it.

Would you like to submit your changes as a PR?

Thinking about it some more, I am not sure if my proposed fix actually calls aclose() and hence adresses the original goal of this MR...

I tried

        async with contextlib.aclosing(self.sample(condition).__aiter__()) as tick:
            async for clk, rst, *values, done in tick:
                if rst:
                    raise DomainReset
                if done:
                    return tuple(values)

but ran into the same issues that the manual handlin in master is having...

whitequark · 2025-06-27T09:31:27Z

Truth be told, I don't actually understand the semantics of / need for aclose here...

whitequark · 2025-06-27T09:51:25Z

I don't need to build a simplified test case to generate it.

Not to prove something to myself, no, but it might be quite useful to have one for the purpose of understanding this problem better, and also in order to have a regression test.

I think asyncio collects all of the async generators you await and then calls .aclose() on them later or something? Which is something that the Amaranth simulator does not. There might be more than one bug here.

whitequark requested a review from wanda-phi May 5, 2025 04:18

whitequark temporarily deployed to publish May 5, 2025 04:23 — with GitHub Actions Inactive

whitequark added this pull request to the merge queue May 11, 2025

Merged via the queue into amaranth-lang:main with commit f3ac270 May 11, 2025
18 of 19 checks passed

whitequark deleted the sim-aclose branch May 11, 2025 05:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

sim: call `.aclose()` on `TickTrigger` in `.until()` and `.repeat()` #1590

sim: call `.aclose()` on `TickTrigger` in `.until()` and `.repeat()` #1590

Uh oh!

whitequark commented May 5, 2025

Uh oh!

Uh oh!

mabl commented Jun 27, 2025

Uh oh!

whitequark commented Jun 27, 2025

Uh oh!

whitequark commented Jun 27, 2025

Uh oh!

mabl commented Jun 27, 2025

Uh oh!

whitequark commented Jun 27, 2025

Uh oh!

whitequark commented Jun 27, 2025

Uh oh!

Uh oh!

sim: call .aclose() on TickTrigger in .until() and .repeat() #1590

sim: call .aclose() on TickTrigger in .until() and .repeat() #1590

Uh oh!

Conversation

whitequark commented May 5, 2025

Uh oh!

Uh oh!

mabl commented Jun 27, 2025

Uh oh!

whitequark commented Jun 27, 2025

Uh oh!

whitequark commented Jun 27, 2025

Uh oh!

mabl commented Jun 27, 2025

Uh oh!

whitequark commented Jun 27, 2025

Uh oh!

whitequark commented Jun 27, 2025

Uh oh!

Uh oh!

sim: call `.aclose()` on `TickTrigger` in `.until()` and `.repeat()` #1590

sim: call `.aclose()` on `TickTrigger` in `.until()` and `.repeat()` #1590